]> dgit.raspbian.org Git - nodejs.git/commitdiff
http2: retain header memory in session accounting
authorMatteo Collina <hello@matteocollina.com>
Mon, 8 Jun 2026 11:53:02 +0000 (13:53 +0200)
committerBastien Roucariès <rouca@debian.org>
Sun, 16 Aug 2026 17:34:06 +0000 (19:34 +0200)
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/63752
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
CVE-ID: CVE-2026-56846
origin: https://github.com/nodejs/node/commit/f14d78b9e0201bfe0291f2b0dc4b5d88c70cc28e
bug: https://nodejs.org/en/blog/vulnerability/july-2026-security-releases#http2-retained-headers-can-bypass-maxsessionmemory-limits-cve-2026-56846---high

Gbp-Pq: Topic sec
Gbp-Pq: Name CVE-2026-56846.patch

doc/api/http2.md
src/node_http2.cc
src/node_http2.h
test/parallel/test-http2-max-session-memory-stalled-headers.js [new file with mode: 0644]

index 705644d5c742d0f6f70fb31d4617b19a27a74654..bc63db0dccb9d967b129f1d650e0e7a2da995244 100644 (file)
@@ -2427,9 +2427,10 @@ changes:
     This is a credit based limit, existing `Http2Stream`s may cause this
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
-    the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
-    counted towards the current limit. **Default:** `10`.
+    the current memory use of the header compression tables, header blocks
+    retained by open streams, current data queued to be sent, and
+    unacknowledged `PING` and `SETTINGS` frames are all counted towards the
+    current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     This is similar to [`server.maxHeadersCount`][] or
     [`request.maxHeadersCount`][] in the `node:http` module. The minimum value
@@ -2584,9 +2585,10 @@ changes:
     credit based limit, existing `Http2Stream`s may cause this
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
-    the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
-    counted towards the current limit. **Default:** `10`.
+    the current memory use of the header compression tables, header blocks
+    retained by open streams, current data queued to be sent, and
+    unacknowledged `PING` and `SETTINGS` frames are all counted towards the
+    current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     This is similar to [`server.maxHeadersCount`][] or
     [`request.maxHeadersCount`][] in the `node:http` module. The minimum value
@@ -2712,9 +2714,10 @@ changes:
     This is a credit based limit, existing `Http2Stream`s may cause this
     limit to be exceeded, but new `Http2Stream` instances will be rejected
     while this limit is exceeded. The current number of `Http2Stream` sessions,
-    the current memory use of the header compression tables, current data
-    queued to be sent, and unacknowledged `PING` and `SETTINGS` frames are all
-    counted towards the current limit. **Default:** `10`.
+    the current memory use of the header compression tables, header blocks
+    retained by open streams, current data queued to be sent, and
+    unacknowledged `PING` and `SETTINGS` frames are all counted towards the
+    current limit. **Default:** `10`.
   * `maxHeaderListPairs` {number} Sets the maximum number of header entries.
     This is similar to [`server.maxHeadersCount`][] or
     [`request.maxHeadersCount`][] in the `node:http` module. The minimum value
index c2de4a3e5f57ede613217911745941e5ddd6f68d..2e7745f82f525593b738b7331575f454b99d55b7 100644 (file)
@@ -768,6 +768,14 @@ BaseObjectPtr<Http2Stream> Http2Session::RemoveStream(int32_t id) {
   stream = FindStream(id);
   if (stream) {
     streams_.erase(id);
+    if (stream->current_headers_length_ > 0) {
+      DecrementCurrentSessionMemory(stream->current_headers_length_);
+      stream->current_headers_length_ = 0;
+    }
+    if (stream->retained_headers_length_ > 0) {
+      DecrementCurrentSessionMemory(stream->retained_headers_length_);
+      stream->retained_headers_length_ = 0;
+    }
     DecrementCurrentSessionMemory(sizeof(*stream));
   }
   return stream;
@@ -1394,7 +1402,10 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
   });
   CHECK_EQ(stream->headers_count(), 0);
 
-  DecrementCurrentSessionMemory(stream->current_headers_length_);
+  // Keep the header block charged against maxSessionMemory while the
+  // corresponding JS objects can still keep it alive for the lifetime of
+  // the stream.
+  stream->retained_headers_length_ += stream->current_headers_length_;
   stream->current_headers_length_ = 0;
 
   Local<Value> args[] = {
index 87f6ab8305a7d85c1dfa7f048d6465aef570767a..f1d8d4d743209f52835549037c810a6f5a8ae07a 100644 (file)
@@ -485,9 +485,11 @@ class Http2Stream : public AsyncWrap,
 
   // The Current Headers block... As headers are received for this stream,
   // they are temporarily stored here until the OnFrameReceived is called
-  // signalling the end of the HEADERS frame
+  // signalling the end of the HEADERS frame.
   nghttp2_headers_category current_headers_category_ = NGHTTP2_HCAT_HEADERS;
   uint32_t current_headers_length_ = 0;  // total number of octets
+  // Charged against maxSessionMemory while headers stay alive in JS.
+  uint64_t retained_headers_length_ = 0;
   std::vector<Http2Header> current_headers_;
 
   // This keeps track of the amount of data read from the socket while the
diff --git a/test/parallel/test-http2-max-session-memory-stalled-headers.js b/test/parallel/test-http2-max-session-memory-stalled-headers.js
new file mode 100644 (file)
index 0000000..8cd8ac0
--- /dev/null
@@ -0,0 +1,69 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const Countdown = require('../common/countdown');
+const assert = require('assert');
+const http2 = require('http2');
+
+const {
+  NGHTTP2_ENHANCE_YOUR_CALM,
+} = http2.constants;
+
+// Regression test: header blocks retained by stalled streams should continue
+// to count against maxSessionMemory after they have been handed to JS.
+const maxSessionMemory = 1;
+const totalRequests = 400;
+const cookieCrumbs = 120;
+
+let accepted = 0;
+let rejected = 0;
+
+const server = http2.createServer({ maxSessionMemory });
+server.on('stream', (stream) => {
+  accepted++;
+  stream.on('error', () => {});
+  stream.respond();
+  stream.write('x');
+});
+
+server.listen(0, common.mustCall(() => {
+  const client = http2.connect(`http://localhost:${server.address().port}`, {
+    settings: {
+      initialWindowSize: 0,
+    },
+  });
+  client.on('error', () => {});
+
+  client.on('remoteSettings', common.mustCall(() => {
+    let destroyed = false;
+    const countdown = new Countdown(totalRequests, common.mustCall(() => {
+      assert(rejected > 0);
+      assert(accepted < totalRequests);
+      server.close();
+    }));
+
+    for (let i = 0; i < totalRequests; i++) {
+      const headers = {':path': '/', 'cookie': []};
+      for (let j = 0; j < cookieCrumbs; j++) {
+        headers.cookie.push('a=1');
+      }
+
+      const req = client.request(headers);
+      req.on('error', () => {});
+      req.on('close', () => {
+        if (req.rstCode === NGHTTP2_ENHANCE_YOUR_CALM) {
+          rejected++;
+          if (!destroyed) {
+            destroyed = true;
+            client.destroy();
+          }
+        }
+        countdown.dec();
+      });
+      req.end();
+    }
+  }));
+}));